Emit lockfile v2 and fix bin links with NPM v7+ #302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lilyinstarlight
force-pushed
the
fix/npmv7
branch
from
bdc1af8
to
f1ecc51
Compare
|
Okay, I think I've got this rebuilding the This should be ready for review/merge and I will be making a companion PR in nixpkgs in a few that brings the |
Lockfile v2 mostly just has a bit of extra metadata and all dependencies are hoisted to the top-level with path-specific keys in a new lock value called "packages". This update emits enough of the format that NPM v7+ seem to be happy enough with it and does not try to rewrite it and cause ENOTCACHED errors with the sandbox. As of NPM v7+, it no longer links bins for the top-level project automatically unless a global install is selected[1][2]. Given a global install would cause more problems than it would solve, I added a simple script to perform the linking ourselves and instructed `npm install` to never link them for consistency. Closes svanderburg#236, svanderburg#293, svanderburg#294 [1]: npm/cli@e46400c#diff-24c01909dabbe2fc000fb5b43d14b511fb335b2f0c2e8e7a671f7d567a33d577R17-R18 [2]: npm/cli#4308
lilyinstarlight
force-pushed
the
fix/npmv7
branch
from
f1ecc51
to
375a055
Compare
13 tasks
|
Opened PR in nixpkgs as NixOS/nixpkgs#193337 |
|
Ok, nice contribution! I'll merge it |
|
What version of node2nix is coming out as? |
ryuheechul
added a commit
to ryuheechul/dotfiles
that referenced
this pull request
Oct 1, 2022
lilyinstarlight
added a commit
to lilyinstarlight/nixpkgs
that referenced
this pull request
Mar 7, 2023
This is minor fallout from svanderburg/node2nix#302 which made node2nix handle installing script bins itself (since npm stopped doing that correctly). It seems npm was doing line-ending normalization when installing these files itself, so in addition to making all bins executable we now also do a crlf to lf conversion on them if they are a script with a shebang.
github-actions bot
pushed a commit
to NixOS/nixpkgs
that referenced
this pull request
Mar 9, 2023
This is minor fallout from svanderburg/node2nix#302 which made node2nix handle installing script bins itself (since npm stopped doing that correctly). It seems npm was doing line-ending normalization when installing these files itself, so in addition to making all bins executable we now also do a crlf to lf conversion on them if they are a script with a shebang. (cherry picked from commit b85c1e3)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Lockfile v2 mostly just has a bit of extra metadata and all dependencies are hoisted to the top-level with path-specific keys in a new lock value called "packages". This update emits enough of the format that NPM v7+ seem to be happy enough with it and does not try to rewrite it and cause
ENOTCACHEDerrors with the sandbox.As of NPM v7+, it no longer links bins for the top-level project automatically unless a global install is selected1 (and is not a problem specific to node2nix2). Given a global install would cause more problems than it would solve, I added a simple script to perform the linking ourselves and instructed
npm installto never link them for consistency.Additionally, this adds a
postRebuildhook that can be used by packages to run extra build scripts as needed by the project before thenpm installstep prunes dev dependencies (also seems to be a new behavior of NPM v7+, but admittedly this behavior could be due to the fake lock file confusing it a little)Closes #236, closes #293, closes #294
Edit:
Marking as draft until I can get this to work completely with thenodePackagesattrset in nixpkgs